fix: Exclude cache files from rules compilation #5283
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
## Summary
Prevents system files like `.DS_Store`, `.Thumbs.db`, and other cache files from being processed as rule files in the `.roo/rules-{mode-slug}` directories.
## Changes
- Added `shouldIncludeRuleFile()` function to filter out unwanted files during rule compilation
- Modified `readTextFilesFromDirectory()` to apply file filtering before processing
- Added support for 21 common cache file patterns including `.DS_Store`, `.db`, `.bak`, `.log`, etc.
- Updated comment to reflect the new filtering behavior
## Impact
- Fixes issue where macOS `.DS_Store` files were being included in system prompts
- Prevents other cache and temporary files from polluting rule compilation
- Improves system prompt quality and reduces noise
## Additional Notes
**Why we didn't import from existing exclusion patterns:**
The existing `getCacheFilePatterns()` function in `src/services/checkpoints/excludes.ts` contains identical patterns, but we cannot import from it due to architectural constraints:
- The `custom-instructions.ts` file is imported by webview components that run in the browser
- `excludes.ts` has Node.js dependencies (`fs/promises`, `path`) that cannot be bundled for browser environments
- Importing from `excludes.ts` would cause Vite build failures in the webview
This is a necessary architectural trade-off to maintain browser compatibility while solving the cache file inclusion issue.
Resolves: RooCodeInc#2728
dosubot
bot
added
size:M
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
Co-authored-by: ellipsis-dev[bot] <65095814+ellipsis-dev[bot]@users.noreply.github.com>
hannesrudolph
added
the
Issue/PR - Triage
…tion ## Summary Addresses feedback by adding comprehensive test coverage for cache file filtering and simplifying the filtering logic using functional programming patterns. ## Changes - **Test Coverage**: Added `"should filter out cache files from .roo/rules/ directory"` test case - Tests filtering of `.DS_Store`, `Thumbs.db`, `.bak`, `.log`, `.tmp`, `.pyc` files - Verifies rule files are still processed correctly - Confirms cache files are completely excluded from processing - Prevents future regressions in filtering logic - **Code Simplification**: Refactored `shouldIncludeRuleFile()` function - Replaced for loop with `.some()` method for more concise logic - Improved functional programming style while maintaining identical behavior - Enhanced code readability and maintainability ## Impact - ✅ 25/27 tests passing with comprehensive cache file filtering coverage - ✅ More maintainable and testable codebase - ✅ Prevents future regressions in rule compilation filtering - ✅ Addresses all reviewer feedback points ## Technical Notes - Test uses mock file system to simulate various cache file types - Verifies that `readFileMock` is never called for filtered cache files - Maintains cross-platform compatibility (Windows/Unix path handling)
dosubot
bot
added
size:L
hannesrudolph
added
PR - Needs Preliminary Review
and removed
Issue/PR - Triage
Member
|
Thank you @MuriloFP I updated the implementation to use an allowlist approach instead of maintaining a blocklist of files to exclude. Now it only accepts known rule file extensions: This is cleaner and more predictable. We define exactly what we want, rather than trying to try and guess everything we don't. It also solves the original The tests have been updated to reflect this change, and all CI checks are passing. |
mrubens
reviewed
Jul 1, 2025
daniel-lxs
force-pushed
the
fix-filter-rules-files
branch
from
eea2620 to
cbf0c99
Compare
daniel-lxs
approved these changes
Jul 1, 2025
Member
daniel-lxs
left a comment
daniel-lxs
left a comment
There was a problem hiding this comment.
@mrubens
I reverted the changes
LGTM
daniel-lxs
moved this from PR [Needs Prelim Review]
to PR [Needs Review]
in Roo Code Roadmap
mrubens
approved these changes
Jul 1, 2025
github-project-automation
bot
moved this from PR [Needs Review]
to Done
in Roo Code Roadmap
hannesrudolph
pushed a commit
that referenced
this pull request
Jul 3, 2025
Co-authored-by: ellipsis-dev[bot] <65095814+ellipsis-dev[bot]@users.noreply.github.com>
chrarnoldus
added a commit
to Kilo-Org/kilocode
that referenced
this pull request
Jul 4, 2025
utarn
pushed a commit
to modelharbor/ModelHarbor-Agent
that referenced
this pull request
Jul 4, 2025
Co-authored-by: ellipsis-dev[bot] <65095814+ellipsis-dev[bot]@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Related GitHub Issue
Closes: #2728
Description
Summary
Prevents system files like
.DS_Store,.Thumbs.db, and other cache files from being processed as rule files in the.roo/rules-{mode-slug}directories.Changes
shouldIncludeRuleFile()function to filter out unwanted files during rule compilationreadTextFilesFromDirectory()to apply file filtering before processing.DS_Store,.db,.bak,.log, etc.Impact
.DS_Storefiles were being included in system promptsTest Procedure
Manual Testing
On the current main branch, we see the content from the .DS_Store and other files being added, there's no filter.
With this implementation, the files are filtered and not included in the rules.
"should filter out cache files from .roo/rules/ directory"test case.DS_Store,Thumbs.db,.bak,.log,.tmp,.pycfilesPre-Submission Checklist
Screenshots / Videos
Documentation Updates
Additional Notes
Why we didn't import from existing exclusion patterns:
The existing
getCacheFilePatterns()function insrc/services/checkpoints/excludes.tscontains identical patterns, but we cannot import from it due to architectural constraints:custom-instructions.tsfile is imported by webview components that run in the browserexcludes.tshas Node.js dependencies (fs/promises,path) that cannot be bundled for browser environmentsexcludes.tswould cause Vite build failures in the webviewThis is a necessary architectural trade-off to maintain browser compatibility while solving the cache file inclusion issue.
Let me know if any other changes are needed, or if I should add tests for the filtering logic.
Get in Touch
@MuriloFP
Important
Adds
shouldIncludeRuleFile()to filter out cache files from rule compilation incustom-instructions.ts, improving prompt quality.shouldIncludeRuleFile()incustom-instructions.tsto exclude cache files like.DS_Store,.Thumbs.db, etc., from rule compilation.readTextFilesFromDirectory()to useshouldIncludeRuleFile()for filtering files before processing..DS_Storefiles being included in system prompts.This description was created by
for ec62e2f. You can customize this summary. It will automatically update as commits are pushed.